Skip to content

fix(frontend): resolve the npm-audit security gate - #782

Open
ioanalytica wants to merge 1 commit into
karanhudia:mainfrom
ioanalytica:fix/frontend-npm-audit
Open

fix(frontend): resolve the npm-audit security gate#782
ioanalytica wants to merge 1 commit into
karanhudia:mainfrom
ioanalytica:fix/frontend-npm-audit

Conversation

@ioanalytica

@ioanalytica ioanalytica commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

The scheduled Security Scan (Frontend Security / npm audit) fails on two production advisories. This resolves both.

dompurify (GHSA-c2j3-45gr-mqc4)

dompurify was pinned at 3.4.11, the last affected version. Bumped to 3.4.12, the published fix.

react-router — "RSC Mode CSRF Bypass" (GHSA-qwww-vcr4-c8h2)

The advisory affects React Router's RSC mode. This app is a Vite SPA rendered through <BrowserRouter> (frontend/src/main.tsx) and never uses RSC, so the vector does not apply. The only published fix is react-router@8.3.0 (a v7→v8 major upgrade), and npm's automatic remediation is a breaking downgrade to 7.11.0. Rather than take either, it is tracked as a reviewed, non-applicable advisory.

npm-audit allowlist

npm audit has no --ignore-vuln, so scripts/npm_audit_known_vulns.py gates the audit JSON against security/npm-audit-known-vulns.json — mirroring the existing pip-audit known-vulns approach. Every entry carries a reason, reviewed_at, and source, so the allowlist is an audited record rather than a silent mute. security.yml's npm-audit step now runs the gate. Adds tests/unit/test_npm_audit_known_vulns.py.

Summary by CodeRabbit

  • Security
    • Enhanced dependency vulnerability scanning to omit development packages and use structured audit output.
    • Added allowlisted advisory gating so only non-allowlisted issues at or above the configured severity fail the scan.
    • Updated the tracked allowlist with a reviewed advisory and aligned severity handling accordingly.
  • New Features
    • Updated the frontend HTML sanitization library to a newer patched version.
  • Tests
    • Added unit tests covering severity filtering, allowlist validation, advisory parsing, and gate behavior.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9d57e686-575a-480c-9487-da0b05310547

📥 Commits

Reviewing files that changed from the base of the PR and between 4fd5bca and 7130542.

⛔ Files ignored due to path filters (1)
  • frontend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • .github/workflows/security.yml
  • frontend/package.json
  • scripts/npm_audit_known_vulns.py
  • security/npm-audit-known-vulns.json
  • tests/unit/test_npm_audit_known_vulns.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • security/npm-audit-known-vulns.json
  • frontend/package.json
  • .github/workflows/security.yml

📝 Walkthrough

Walkthrough

The security workflow now produces JSON npm audit results and gates moderate-or-higher advisories through a Python allowlist checker. The allowlist records one react-router advisory, with unit tests covering extraction, filtering, and validation. dompurify is pinned to 3.4.12.

Changes

Security audit gating

Layer / File(s) Summary
Audit allowlist gate
scripts/npm_audit_known_vulns.py
Adds severity filtering, GHSA extraction, allowlist and report validation, CLI handling, and exit-status gating for npm audit reports.
Workflow allowlist and validation
.github/workflows/security.yml, security/npm-audit-known-vulns.json
The workflow invokes JSON audit gating, and the allowlist records the react-router advisory.
Audit gate tests
tests/unit/test_npm_audit_known_vulns.py
Tests cover advisory extraction, filtering, allowlist validation, report validation, and gate exit statuses.

Frontend dependency pin

Layer / File(s) Summary
dompurify override
frontend/package.json
Updates the dompurify override from 3.4.11 to 3.4.12.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant npmAudit
  participant npm_audit_known_vulns.py
  participant KnownVulnsAllowlist
  GitHubActions->>npmAudit: Generate npm-audit.json
  GitHubActions->>npm_audit_known_vulns.py: Check moderate findings
  npm_audit_known_vulns.py->>KnownVulnsAllowlist: Load known advisory IDs
  npm_audit_known_vulns.py-->>GitHubActions: Return pass or fail status
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing the frontend npm-audit security gate.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/npm_audit_known_vulns.py`:
- Around line 38-44: Update load_known_vulns in scripts/npm_audit_known_vulns.py
(lines 38-44) to require reason to be a non-empty string after trimming,
rejecting missing, non-string, and whitespace-only values with ValueError. Add
an ID-only fixture and assert loading raises ValueError in
tests/unit/test_npm_audit_known_vulns.py (lines 69-73).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 149972cc-d58c-493d-b0d0-cc1300951e82

📥 Commits

Reviewing files that changed from the base of the PR and between 380827d and 2d6d489.

⛔ Files ignored due to path filters (1)
  • frontend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • .github/workflows/security.yml
  • frontend/package.json
  • scripts/npm_audit_known_vulns.py
  • security/npm-audit-known-vulns.json
  • tests/unit/test_npm_audit_known_vulns.py

Comment thread scripts/npm_audit_known_vulns.py
@ioanalytica
ioanalytica force-pushed the fix/frontend-npm-audit branch from 2d6d489 to 4fd5bca Compare July 27, 2026 13:50
@karanhudia

Copy link
Copy Markdown
Owner

Nice approach overall, and the React Router RSC exception makes sense for this app. One thing I’d fix before merging: the new gate currently treats a valid npm error response with no vulnerabilities field as clean, because the workflow ignores the audit exit code and the parser defaults missing vulnerabilities to an empty set. Could we make it fail on error/unexpected report shapes and add a small regression test? Once that’s covered, this looks good to me.

Two production advisories were failing the scheduled Security Scan:

- dompurify <=3.4.11 (GHSA-c2j3-45gr-mqc4): bumped to 3.4.12, the published fix.
- react-router "RSC Mode CSRF Bypass" (GHSA-qwww-vcr4-c8h2): this app is a Vite
  SPA using <BrowserRouter>, not React Router's RSC mode, so the vector does not
  apply, and the only published fix is a v7->v8 major upgrade. Track it as a
  reviewed, non-applicable advisory rather than downgrade to 7.11.0.

npm audit has no --ignore-vuln, so scripts/npm_audit_known_vulns.py gates the
audit JSON against security/npm-audit-known-vulns.json — mirroring the existing
pip-audit known-vulns approach. Every allowlist entry carries a reason and a
review date, so it is an audited record, not a silent mute.
@ioanalytica
ioanalytica force-pushed the fix/frontend-npm-audit branch from 4fd5bca to 7130542 Compare July 29, 2026 05:12
@ioanalytica

Copy link
Copy Markdown
Contributor Author

Good catch — confirmed and fixed in 7130542.

The gate now validates the report shape before deciding: an npm error payload ({"error": ...}), an empty/unparseable file, or a report without a vulnerabilities object now fails the gate instead of parsing into zero advisories and passing as clean. The parser no longer defaults missing vulnerabilities to an empty set — it indexes it directly on a validated report.

Regression tests cover all of those failure modes (error payload, missing vulnerabilities, non-object, empty output), plus a genuinely-clean {"vulnerabilities": {}} still passing. I also documented in the workflow why || true is safe now: npm exits non-zero on any advisory, so the gate — not the exit code — is the checkpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants